feat(stream): add natural time units support for PERIOD trigger#34766
feat(stream): add natural time units support for PERIOD trigger#34766JinqingKuang wants to merge 1 commit into3.0from
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the stream processing capabilities by introducing natural time units (weeks, months, years) for Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces support for natural time units (week, month, year) in the PERIOD trigger for streams, which is a valuable enhancement for usability. The implementation is comprehensive, including updates to the core time alignment logic, parser validation, data serialization, and extensive testing across unit, integration, and system levels. The documentation has also been updated clearly. I have a few suggestions to improve code clarity, robustness, and test hygiene.
test/cases/18-StreamProcessing/03-TriggerMode/test_period_natural_units.py
Show resolved
Hide resolved
9d70529 to
c1eb39e
Compare
c1eb39e to
fca9717
Compare
e5f65db to
b4fe192
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for natural time units (week, month, year) for the PERIOD trigger, which is a valuable feature enhancement. The changes are comprehensive, covering documentation, parser validation, core time alignment logic, and message serialization. The addition of extensive unit and end-to-end tests is particularly commendable and ensures the feature's reliability. My review includes one high-severity suggestion to improve the robustness of the time alignment logic by adding error handling and a couple of medium-severity suggestions to enhance the clarity of the user documentation.
b4fe192 to
4552305
Compare
Implement week/month/year units for stream PERIOD trigger with natural boundary alignment and offset support. Key changes: - Parser: Add validation for natural time units (w/n/y) and offset parameter - Time utilities: Add getDuration() support for week/month/year units - TriggerTask: Implement window calculation with natural boundary alignment - Week: align to Monday 00:00:00 - Month: align to 1st of month 00:00:00 - Year: align to Jan 1st 00:00:00 - Add offset support: PERIOD(1w, 1d) shifts window by 1 day - Unit tests: Parser validation, time utilities, TriggerTask window calculation - System tests: End-to-end tests for week/month/year units with offset - Documentation: Update user manual with natural time unit examples Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
4552305 to
a25590f
Compare
There was a problem hiding this comment.
Pull request overview
Adds support for natural calendar units (week/month/year) to stream PERIOD triggers, including natural boundary alignment and an optional offset, and wires the new metadata through parser/runtime plus tests/docs.
Changes:
- Extend PERIOD parsing/validation to accept
w/n/yunits and validateoffset < period(with month validation based on 28 days). - Implement natural-boundary alignment in time utilities and trigger-task window calculation for week/month/year (+ offset).
- Add unit/system tests and update user docs + error codes for new validation errors.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/ci/cases.task | Adds CI entry for the new PERIOD natural units system test. |
| test/cases/18-StreamProcessing/20-UseCase/tobacco_data/idmp/stream.json | Adjusts an existing PERIOD offset to satisfy stricter validation. |
| test/cases/18-StreamProcessing/03-TriggerMode/test_period_natural_units.py | New system test coverage for w/n/y PERIOD units and offsets. |
| source/util/src/terror.c | Adds error strings for invalid offset unit/value. |
| include/util/taoserror.h | Adds new parser error codes for offset validation. |
| docs/en/14-reference/09-error-code.md | Documents new error codes (EN). |
| docs/zh/14-reference/09-error-code.md | Documents new error codes (ZH). |
| source/libs/parser/src/parTranslater.c | Parser-side PERIOD unit/range + offset validation logic updates. |
| source/libs/parser/test/parStreamTest.cpp | Adds/updates parser tests for PERIOD ranges and offsets. |
| include/common/ttime.h | Exposes alignToNaturalBoundary() API. |
| source/common/src/ttime.c | Implements alignToNaturalBoundary() and adds getDuration() support for week. |
| source/common/test/ttimeNaturalUnitsTest.cpp | Adds unit tests for natural boundary alignment + duration conversion. |
| source/common/test/CMakeLists.txt | Registers the new common unit test binary. |
| source/libs/new-stream/src/streamTriggerTask.c | Implements natural-unit window calculation + exports helper functions. |
| source/libs/new-stream/inc/streamTriggerTask.h | Exposes trigger-task window helper functions for testing/usage. |
| source/libs/new-stream/test/streamTriggerTaskTest.cpp | Adds trigger-task window unit tests for natural units/offsets. |
| source/libs/new-stream/test/CMakeLists.txt | Adds the new new-stream unit test target. |
| source/common/src/msg/streamMsg.c | Extends stream trigger deploy message encoding/decoding for PERIOD metadata. |
| include/util/tringbuf.h | Refactors internal ring buffer type aliasing/casts for general use. |
| docs/en/14-reference/03-taos-sql/41-stream.md | Updates PERIOD docs with natural unit behavior/examples (EN). |
| docs/zh/14-reference/03-taos-sql/41-stream.md | Updates PERIOD docs with natural unit behavior/examples (ZH). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Implement week/month/year units for stream PERIOD trigger with natural boundary alignment and offset support.
Key changes:
Issue(s)
Checklist
Please check the items in the checklist if applicable.